Emit calls edges for top-level function calls, with file node as caller#2016
Emit calls edges for top-level function calls, with file node as caller#2016Rishet11 wants to merge 2 commits into
Conversation
… as caller (Graphify-Labs#1972) Calls at a file's top level (or inside a plain DSL block) had no enclosing definition, so they never entered function_bodies and got no caller node or edge. Walk the file root once with the file node as caller after the per-definition walk; walk_calls already returns at every definition boundary, so it cannot double-emit anything the per-body loop covered. During this root walk emit ONLY direct calls: a _toplevel_calls_only flag suppresses indirect_call (already handled by the dedicated module-dispatch pass with correct shadow filtering) and the PHP-family reference emissions. Java is skipped entirely (no top-level statements; field initializers are walked via initializer_nodes with the owning class as caller). Covers Python and JS/TS, which share this extractor. Adds tests for top-level Python/JS calls and an in-context regression guard.
There was a problem hiding this comment.
Pull request overview
This PR fixes a graph-extraction gap where calls made at a file’s top level (outside any function/method body) previously had no caller node and therefore emitted no calls edges. It does so by adding a second walk_calls pass rooted at the file AST root with the file node as caller_nid, while gating that pass to avoid duplicating non-calls relations handled by other module-scope passes.
Changes:
- Add a file-root
walk_callspass (skipping Java) to emitcallsedges for script/top-level call sites (#1972). - Introduce a
_toplevel_calls_onlyguard to suppress indirect-dispatch and other non-callsrelations during the file-root walk. - Add tests covering top-level Python calls, top-level JS calls, and a regression guard ensuring no duplicate/incorrect caller attribution.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| graphify/extractors/engine.py | Adds a guarded file-root call walk to attribute top-level calls to the file node and avoid duplicating non-calls relations. |
| tests/test_extract.py | Adds regression tests ensuring top-level calls now produce calls edges sourced from the file node (and that in-function calls remain unchanged). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@Rishet11 Thanks for picking this up, this is exactly the shape we hoped for: file node as caller, with the walk returning at definition boundaries so nothing double-emits. One coverage ask from the corpus that motivated #1972: our worst case is Ruby rake files, where every call lives either at the file root or inside a On scoping: agreed that limiting the extra walk to direct calls is right for this PR. For Ruby, constant reads are missing everywhere, not just at top level, so that half of #1972 is bigger than this change anyway. Fine by us if it stays tracked on the issue after this merges. Offer: we run a recall census against a real Rails corpus (same one behind our earlier graphify reports). Happy to run it against a build of your branch and report before/after edge counts if useful. |
Fixes #1972
Problem
Calls made at a file's top level (outside any def) have no caller node, so they emit no
callsedges. A function invoked only from module scope looks dead in the graph.Fix
In
graphify/extractors/engine.py, after the per-definition walk, walk the file root once more with the file node as caller.walk_callsalready returns at every definition boundary, so nothing already covered gets re-emitted.Two scoping decisions:
_toplevel_calls_onlyflag limits the extra walk to directcalls. Indirect dispatch and PHP-family references have their own passes; emitting them here would duplicate work with empty shadow context.Test
3 new tests: top-level Python call, top-level JS call, and an in-function regression guard. Full suite shows no new failures against the v8 baseline.
One known behavior to flag: class-body statements now get attributed to the file (relevant for Swift field initializers). No test covers that yet, so I left it alone for now.
Still fairly new to this codebase, so if I have the approach or the scoping wrong anywhere, tell me and I will fix it up.